Skip to content

refactor: leave precompute/query placement to the lifecycle layer - #485

Open
zzylol wants to merge 3 commits into
feat/compile-once-cutsfrom
refactor/lifecycle-only-placement
Open

zzylol wants to merge 3 commits into
feat/compile-once-cutsfrom
refactor/lifecycle-only-placement

Conversation

@zzylol

@zzylol zzylol commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #479.

Rebuilt into the linear stack on main. Conflict resolutions are recorded in the messages of: "fix: keep a selected grouped Sum over realized Rate readouts".

Why

The agreed layering says logical PlanSpace decides what to compute and a
chosen lifecycle assignment decides timing. Two logical-side sites still chose
placement for grouped Rate→Sum and Rate heap Top-K:

What

  • Remove both placement-variant methods.
  • Assembly keeps a selected Sum summary that realizes its inner aggregate. The
    query-time residual (fix(sql): preserve nested temporal aggregates #372) still applies when the outer summary would hide the
    inner aggregate in KeepPreAsap.
  • promql_rows::compile_fixed_window_rate_aggregation now takes the
    lifecycle-timed PostAsapDag.
  • Comments mark the remaining strategy timings as initial layout; design docs
    describe Candidates A/B as lifecycle choices.

Before this PR

sum by(job)(rate(m[1m])) inventory: raw | Rate state → query-time exact Sum
Sum in precompute: only via fixed_window_rate_candidates (timing-flipped copy)

After this PR

inventory: raw | Rate state → query-time exact Sum | Rate state → Rate readout → Sum state
Rate CM + Sum CM        → precompute: Rate + Sum builds;  query: Sum readout
Rate CM + Sum Ephemeral → precompute: Rate build;         query: Rate readout → Sum build → readout

Default global_selection for grouped Rate→Sum now returns the Sum-state
candidate instead of the query-time residual. precompute_candidates reads its
query root (assemble_selected_query) and finds the Rate readout by edge. Every
other fixture is unchanged.

Not in scope

  • Maintained populations (MaintainPopulation): execution_timed_dag refuses
    them until lifecycle enumeration covers non-SummaryAgg state.
  • Deriving the precompute frontier from timing inside the physical planner.
    Explicit compile_candidates frontiers can still persist per-series rate
    values.
  • The lifecycle layer does not yet tell a per-window rebuild apart from
    incremental maintenance for rate-snapshot heaps/Sums. The comment states the
    "no accumulation across evaluations" requirement.
  • The query-time exact Sum residual and Sum state + Ephemeral give the same
    placement through different operators. Both remain as logical alternatives.

Validation

  • Written first and failing before the fix:
    grouped_rate_sum_inventory_keeps_sum_state_for_lifecycle_placement (unit) and
    grouped_rate_sum_placement_is_a_lifecycle_choice (e2e, both placements
    compiled to the expected precompute/query contents).
  • Guard: rate_candidate_inventories_have_no_timing_only_duplicates.
  • maintained_rate_heap_lifecycle_compiles_fixed_window_precompute replaces the
    fixed-window heap test and executes across the state boundary.
  • cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings,
    cargo test --workspace --no-fail-fast: 1372 passed, 0 failed.

🤖 Generated with Claude Code

zzylol and others added 3 commits September 30, 2026 18:06
DAG assembly replaced any selected outer Sum over an inner aggregate with a
query-time exact Sum, even when the selected summary realizes the inner Rate
itself. The grouped Sum state therefore never reached the inventory, and no
lifecycle choice could move grouped Sum into precompute.

Assembly now keeps such a selected summary; the query-time residual still
applies when the outer summary would hide its inner aggregate in KeepPreAsap.
Default selection for sum by(job)(rate(...)) now yields Rate -> grouped Sum
state; the physical frontier test reads its query root accordingly.

Conflicts with earlier stack changes resolved to the integration tree:
- crates/integration-tests/tests/summary_maintenance_lifecycle_e2e.rs: c98281a Merge remote-tracking branch 'origin/feat/compile-once-cuts' into integration/planner-for-backend

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SketchAlgorithmStrategy::fixed_window_rate_candidates and
query_time_rate_aggregation_candidates returned the same logical DAG as the
ordinary heap or grouped Sum candidate with Rate finalization flipped between
ingestion and query time. Placement now comes only from a chosen lifecycle via
SummaryMaintenanceLifecyclePlan::execution_timed_dag.

compile_fixed_window_rate_aggregation takes that lifecycle-timed PostAsapDag
instead of a SummaryNode with baked-in timing. The fixed-window heap test binds
continuously maintained lifecycles; the grouped Sum placement pair is covered
by the lifecycle end-to-end test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/lifecycle-timing branch from 0747971 to 3b281da Compare September 30, 2026 18:27
@zzylol
zzylol force-pushed the refactor/lifecycle-only-placement branch from 69e0b0d to eb5885c Compare September 30, 2026 18:27
@zzylol
zzylol changed the base branch from feat/lifecycle-timing to feat/compile-once-cuts September 30, 2026 18:28
zzylol added a commit that referenced this pull request Sep 30, 2026
Describe the two plain candidate collections, the view-based compile, the
caller-typed physical candidate errors, lifecycle_guarantee, and
PhysicalExecution as an execution handle. Remove APIs the docs said were
removed but never existed (compile_timed_candidates, PhysicalDAGCandidate),
the agent instructions in the alignment proposal's baseline, and the
ingestion-time Binary exception from design docs, where it is an
implementation detail (the developer migration guide keeps it).

Restore #485's statement that candidates do not choose placement and #508's
CandidatePostASAPDAGs<Id> names in input-output-workflow.md, rejoin the
split test table in physical-planning-and-deployment.md, and take
planner-backend-layering.md verbatim from #509.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant